Skip to content

feat: demonstrate contract-first CRDs #1524

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 7, 2022

Conversation

derlin
Copy link
Contributor

@derlin derlin commented Oct 6, 2022

Modify the leader-election sample to use contract-first. That is, the CRDs is written in YAML and the java code is generated automatically using the java-generator-maven-plugin from fabric8.

#1338

Modify the leader-election sample to use contract-first. That is,
the CRDs is written in YAML and the java code is generated automatically
using the java-generator-maven-plugin from fabric8.
Copy link
Collaborator

@csviri csviri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @derlin , the tests are failing it seems that the CRD is not applied, could you take a look pls?

@derlin
Copy link
Contributor Author

derlin commented Oct 6, 2022

I missed lots of details, sorry for that. Should be fixed now !

@@ -0,0 +1,34 @@
# Custom Resource Definition that will be used to generate the Java classes in target/generated-sources/java
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this probably isn't a big deal, I think that if we try to follow the CRD generator file name format here, then this file should be named with leaderelections (plural form) instead of leaderelection (singular).

Copy link
Collaborator

@csviri csviri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@metacosm metacosm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@metacosm metacosm merged commit f6b16e9 into operator-framework:main Oct 7, 2022
@derlin
Copy link
Contributor Author

derlin commented Oct 7, 2022

@metacosm Great thanks! May I kindly ask to add the label hacktoberfest-accepted to it?

@derlin derlin deleted the 1338-contract-first-crd branch October 7, 2022 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants